Skip to content

Add animation playback preview to scene import settings - #76367

Merged
YuriSizov merged 1 commit into
godotengine:masterfrom
Chaosus:import_settings_animation_playback
Jul 12, 2023
Merged

Add animation playback preview to scene import settings#76367
YuriSizov merged 1 commit into
godotengine:masterfrom
Chaosus:import_settings_animation_playback

Conversation

@Chaosus

@Chaosus Chaosus commented Apr 23, 2023

Copy link
Copy Markdown
Member

This QoL enhancement will add the option to preview animations in the import settings window (to preview animations without performing extra actions such placing the models to a scene):

import_animation

Attached demo project:
TestModel.zip

Bugsquad edit:

@Chaosus
Chaosus requested a review from a team as a code owner April 23, 2023 12:15
@Chaosus Chaosus added this to the 4.1 milestone Apr 23, 2023
@Chaosus
Chaosus force-pushed the import_settings_animation_playback branch 4 times, most recently from 3e2052b to 13df5fa Compare April 23, 2023 19:25
@fire
fire requested a review from a team April 24, 2023 05:05
@clayjohn
clayjohn requested a review from TokageItLab April 24, 2023 18:57
@Ansraer

Ansraer commented Apr 27, 2023

Copy link
Copy Markdown
Contributor

Not sure how hard it would be to implement, but could you add an option to show the bones when rendering an animation?

I am considering to import all my animations from seperate gltf files, so I wouldn't have a mesh for preview purposes during the import.

@jeronimo-schreyer

Copy link
Copy Markdown
Contributor

Not sure how hard it would be to implement, but could you add an option to show the bones when rendering an animation?

I am considering to import all my animations from seperate gltf files, so I wouldn't have a mesh for preview purposes during the import.

godotengine/godot-proposals#6801

@TokageItLab

TokageItLab commented May 25, 2023

Copy link
Copy Markdown
Member

I compared #76367 and #76381, both have good and not good points.

#76367 has a seek bar. It is good that you can check the playback position with it. However, the negative is that the loop option is not linked to the import option for the preview. Since users may complain after importing that it is not in the same state as the preview, they should be linked.

image

#76381 does not have a seek bar, but it is preferable that the loop option for preview be linked to the import option. However, I recommend #76367 for animation because #76367 is more valuable for QOL since it has a seek bar.

BTW, the light settings for preview in #76381 are useful. So it is better to omit the animation feature and brush up #76381 for the preview environment option.

image

@Chaosus
Chaosus force-pushed the import_settings_animation_playback branch from 13df5fa to 32df433 Compare May 25, 2023 10:56
@Chaosus

Chaosus commented May 25, 2023

Copy link
Copy Markdown
Member Author

@TokageItLab I've updated this PR, and now when you click on animation in the list - it will auto-switch the loop button to enabled state (when loop mode is enabled in the animation) and will use play_backward if ping-pong is enabled at the animation end.

I don't think I need to add something more complex like default lightning or bone rendering (which is done by #76818)

@Chaosus
Chaosus force-pushed the import_settings_animation_playback branch from 32df433 to 5e8181e Compare May 25, 2023 11:25
@TokageItLab

TokageItLab commented May 25, 2023

Copy link
Copy Markdown
Member

@Chaosus I understand that those are not necessary for this PR. But I think they can be implemented separately from this PR.

Below are the current problems with this PR:

The loop button in the GUI seems to do nothing, so please get rid of it. Or make sure it is correctly linked to the import option.

image

Also, for ping-pong, it should play to the end, then reverse, and then play forward when it gets to the beginning again. Please refer to the behavior of AnimationPlayer or #76381 for more example on this.

1.mp4

I am also concerned about the presence of gaps at the beginning and end of the seek bar. I don't know if it is a drawing issue or if it is not actually processing the animation, but can it be remedied?

@jeronimo-schreyer

Copy link
Copy Markdown
Contributor

I compared #76367 and #76381, both have good and not good points.

#76367 has a seek bar. It is good that you can check the playback position with it. However, the negative is that the loop option is not linked to the import option for the preview. Since users may complain after importing that it is not in the same state as the preview, they should be linked.

image

#76381 does not have a seek bar, but it is preferable that the loop option for preview be linked to the import option. However, I recommend #76367 for animation because #76367 is more valuable for QOL since it has a seek bar.

BTW, the light settings for preview in #76381 are useful. So it is better to omit the animation feature and brush up #76381 for the preview environment option.

image

sorry, the light settings video sample got mixed up with #76140 but they're a different PR

@akien-mga akien-mga modified the milestones: 4.1, 4.2 Jun 12, 2023
@Chaosus
Chaosus force-pushed the import_settings_animation_playback branch from 5e8181e to 2e36c12 Compare June 12, 2023 14:34
@Chaosus

Chaosus commented Jun 12, 2023

Copy link
Copy Markdown
Member Author

The loop button in the GUI seems to do nothing, so please get rid of it. Or make sure it is correctly linked to the import option.

OK, I deleted it, it was working - but only after you press the play button manually.

Also, for ping-pong, it should play to the end, then reverse, and then play forward when it gets to the beginning again.

It's already being working, as you said. Isn't it?

I am also concerned about the presence of gaps at the beginning and end of the seek bar.

Not sure if I understand how to fix this...

@TokageItLab

Copy link
Copy Markdown
Member

Also, for ping-pong, it should play to the end, then reverse, and then play forward when it gets to the beginning again.

It's already being working, as you said. Isn't it?

No. It is always looped in the backward.

@Chaosus
Chaosus force-pushed the import_settings_animation_playback branch from 2e36c12 to b0e36b8 Compare June 12, 2023 16:14
@Chaosus

Chaosus commented Jun 12, 2023

Copy link
Copy Markdown
Member Author

No. It is always looped in the backward.

OK, Fixed, now. Check again.

@Chaosus
Chaosus force-pushed the import_settings_animation_playback branch from b0e36b8 to 9bb5e42 Compare June 12, 2023 16:16
@TokageItLab

Copy link
Copy Markdown
Member

I have confirmed that it has been fixed.

The remaining problem is that when you change the loop mode, you will need to update the ImportScene or animation for the preview. The current behavior does not update the loop settings until you open the PostImporter again after changing the loop mode and reimport.

@Chaosus
Chaosus force-pushed the import_settings_animation_playback branch from 9bb5e42 to 1d37e94 Compare June 13, 2023 05:36
@Chaosus

Chaosus commented Jun 13, 2023

Copy link
Copy Markdown
Member Author

@TokageItLab I think I fixed this, check again.

@Chaosus
Chaosus force-pushed the import_settings_animation_playback branch from 1d37e94 to 7cd943f Compare June 13, 2023 05:42
Comment on lines +960 to +979
switch (loop_mode) {
case Animation::LOOP_NONE: {
animation_play_button->set_icon(get_theme_icon(SNAME("MainPlay"), SNAME("EditorIcons")));
animation_slider->set_value_no_signal(1.0);
set_process(false);
} break;
case Animation::LOOP_LINEAR: {
animation_player->play(p_name);
} break;
case Animation::LOOP_PINGPONG: {
if (animation_pingpong) {
animation_player->play(p_name);
} else {
animation_player->play_backwards(p_name);
}
animation_pingpong = !animation_pingpong;
} break;
default: {
} break;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems a bit redundant to have to write the processing for looping outside of the AnimationPlayer, but I think this is the only way to do it for now. I will consider providing a better API in a future refactoring planned as godotengine/godot-proposals#5972.

@fire fire left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the design. Did not technical review.

@YuriSizov
YuriSizov merged commit 9547de5 into godotengine:master Jul 12, 2023
@YuriSizov

Copy link
Copy Markdown
Contributor

Thanks!

@Chaosus
Chaosus deleted the import_settings_animation_playback branch July 14, 2023 18:32
BendyLand pushed a commit to BendyLand/voltaire that referenced this pull request Aug 2, 2026
…mation_playback

Add animation playback preview to scene import settings
BendyLand pushed a commit to BendyLand/voltaire that referenced this pull request Aug 2, 2026
…mation_playback

Add animation playback preview to scene import settings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Added playback control to animations in the Advanced Import Settings preview

7 participants